-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/select g house #34
Conversation
src/components/search_list.jsx
Outdated
{props.db.map((guestHouses) => { | ||
const boundItemClick = handleClick.bind(this, guestHouses.name); | ||
return ( | ||
<button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this button should be a different component and then the bind function wont be inside the map function
src/containers/select_guesthouse.jsx
Outdated
currentGuestHouse(guestHouse) { | ||
const current = this.state.db.filter((curr) => { | ||
if (curr.name === guestHouse) { | ||
return curr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curr and current?
@esraajb more like this? 🍁 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good 👍, left a couple of comments. Also, no issue referred
|
||
|
||
class SelectGuesthouse extends Component { | ||
constructor(props) { | ||
super(props); | ||
this.state = { | ||
db: {}, | ||
title: 'PAS', | ||
tagLine: 'Park & Sleep', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are title and tagLine ever change? if not, then its pointless to put them in a state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are going to be changed (header will update based on these values). Next PR.
src/containers/select_guesthouse.jsx
Outdated
db: {}, | ||
title: 'PAS', | ||
tagLine: 'Park & Sleep', | ||
db: [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe instead of db, use 'guestHousesList'? might be clearer for the reader
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good shout
@esraajb ref #22 was in the commit messages 😄 should it be in the comment as well? Changes made. |
Yea, should be in the description |
@esraajb Cool cool, done. And will do going forward. Good to merge? |
ref #22